Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major updates and fixes #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Major updates and fixes #67

wants to merge 2 commits into from

Conversation

nabahr
Copy link

@nabahr nabahr commented Nov 9, 2022

Added availability topic in HA as V1 devices report state every 4 hours at most, and V2 devices report every 2 hours at most. Added a state file that gets saved on shutdown. This file saves the last seen timestamp and online state for each sensor so that it has consistancy across reboots.

Switched mqtt connection method so it connects faster and availability topics are sent after the connection is active rather than before. This also always the main thread to loop and check availability of the sensors.

The main loop also checks communication with the dongle, by getting the mac address, which allows us to close and reopen the dongle if we lose communication with the device. This is an issue I've been dealing with for a long time.

Fixed shutdown by sending the correct signal in the service file and properly handling it in shutdown. Wait for dongle loop to finish before closing the fd.

Updated logging: set sample logging conf to INFO instead of DEBUG and updated log messages to primarily log mqtt messages/events, etc at this level. Passing in LOGGER to the dongle class so logs from the dongle show up in the log file.

Fix packet parsing to not throw away partial packets which can be parsed correctly when the rest of the message is received.

Signed-off-by: Nathan Bahr [email protected]

@raetha
Copy link
Owner

raetha commented Nov 10, 2022

Hi @nabahr. This looks great, and I'm happy to see some activity after so long. Unfortunately I never got any of the v2 stuff that others were working on and was unable to help test or resolve issues in that realm. I'm also a bit rusty on this stuff given how long I've been away from it, so going to request that @drinfernoo and @AK5nowman do code reviews if they see this and have a moment. Otherwise I'll be happy to merge in soon. One suggestion though is to review the readme file as well for potential updates on the new configuration file, and anything that someone might need to know about v2 sensors. Thanks!

@raetha raetha self-requested a review November 10, 2022 22:41
@raetha raetha added bug Something isn't working enhancement New feature or request labels Nov 10, 2022
@nabahr
Copy link
Author

nabahr commented Nov 10, 2022

Yes, there are quite a few changes here that will need some additional testing and I am sure will need some further refinement. I just wanted to throw it out there now to get some feedback.

I've got the original dongle and a bunch of original motion and contact sensors, plus a handful of the new motion and contact sensors. Unfortunately, I do not have any of the other new stuff that may be supported.

@nabahr
Copy link
Author

nabahr commented Nov 11, 2022

So my main goal when I started this was to add the availability topic when I realized that the sensors sent periodic status messages, but it ballooned a bit more than that. My question as far as maintaining history is if you'd want me to go through and break things up into smaller commits or if squashing it all together is fine?

@nabahr
Copy link
Author

nabahr commented Nov 11, 2022

I've gone through and cleaned up a few more little things and I believe it should be ready for others to test.
In all my messing around with hass discovery, I've been able to mess up my mqtt topics and had to wipe out the wyzesense2mqtt and discovery topics and start fresh, but I think that switching to this branch and running it should work fine with no changes required to any config files.

One more thing I've been thinking about adding is a config option to have the entitiy_id automatically updated in hass as well. Looks like setting the object_id instead of name in the discovery config will do that. I may just save that for another PR though.

@drinfernoo
Copy link
Collaborator

Hey, I am planning to do a review on this soon, but I've been pretty dang busy lately myself.

I'll try to remember to take a look in the near future.

@nabahr
Copy link
Author

nabahr commented Nov 18, 2022

Somehow a typo made it in my last push, fixed that. Also added a flag to block the processing of dongle events until everything is initialized.

@nabahr nabahr force-pushed the availability branch 2 times, most recently from 71c99dd to 05523ee Compare December 6, 2022 23:31
Added availability topic in HA as V1 devices report state every
4 hours at most, and V2 devices report every 2 hours at most.
Added a state file that gets saved on shutdown and sensor reload.
This file saves the last seen timestamp and online state for each
sensor so that it has consistancy across reboots. This file also
contains the shutdown timestamp so that we can ignore stale state
data in case the service is shut down for a long time, or if the
service crashes and a fresh state file is not written. If the state
data is stale, the service will just start fresh assuming all devices
are online.

Added timeout setting for sensors to configure a timeout per device
if desired. Otherwise it'll default to 4 hours for V2 devices and
8 hours for V1 and unknown devices. This is twice the max status
period to allow for one missed status update.

Also added a global availability topic that is set online when the
service is running. Both the global availability and the per device
availability must be set to online for a device to be available.

Switched mqtt connection method so it connects faster and availability
topics are sent after the connection is active rather than before.
This also allows the main thread to loop and check availability of
the sensors.

The main loop also checks communication with the dongle, by getting
the mac address, which allows us to close and reopen the dongle if
we lose communication with the device. This is an issue I've been
dealing with for a long time.

Fixed shutdown by sending the correct signal in the service file
and properly handling it in shutdown. Wait for dongle loop to finish
before closing the fd.

Updated logging. Set the sample logging conf to INFO instead of DEBUG
and updated log messages to primarily log mqtt messages/events, etc
at this level. Passing in LOGGER to the dongle class so logs from the
dongle show up in the log file.

Fix packet parsing to not throw away partial packets which can be
parsed correctly when the rest of the message is received.

Fix new device scan to add the device even if the additional calls
timeout, as long as we got a result from the scan.

Fix automatic device adding for already linked devices to default
the class to opening instead of None. This means even motion sensors
will be initially added as a contact sensor, but should get fixed when
the sensors.yaml configuration is updated.

Signed-off-by: Nathan Bahr <[email protected]>
@nabahr
Copy link
Author

nabahr commented Jan 11, 2023

Just wanted to bump this PR and say that I've been running this for a while now with success and the device availability feature has been quite useful because when a device goes offline you have at least a day or two to change the battery and avoid the null mac issue, making the v1 sensors a bit more reliable long term.

Fixed logging in bridge tool cli.
Added some comments and logs to dongle functions.
Increased default timeout for getting sensor list and verifying sensor.
FIxed a few spots where the incorrect mac was used.
Re-added last_seen attribute so that it's updated if the device is
still reporting but hasn't had a state change.
Removed the dongle get mac check. I was still having problems with
my dongle "hanging" every once in a while, not receiving any updates.
I happened to have a second dongle so I switched to that and so far it
has been much more stable. I used to get semi-frequent errors about
mismatched checksums that are much more rare now as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants